Skip to content

appkit: patch a unit test issue in monitor.rs regarding display ID. #4402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Nov 6, 2025
Merged

appkit: patch a unit test issue in monitor.rs regarding display ID. #4402

merged 2 commits into from
Nov 6, 2025

Conversation

@ShikiSuen
Copy link
Contributor

@ShikiSuen ShikiSuen commented Nov 5, 2025

  • Tested on all platforms changed
  • Added an entry to the changelog module if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality

@ShikiSuen ShikiSuen mentioned this pull request Nov 5, 2025
Open
4 tasks
@kchibisov
Copy link
Member

kchibisov commented Nov 5, 2025

Changes to wayland look arbitrary?

@ShikiSuen
Copy link
Contributor Author

ShikiSuen commented Nov 5, 2025

@kchibisov That's what done by cargo +nightly fmt, not me. Lemme remove that commit from this PR.

@ShikiSuen ShikiSuen force-pushed the shiki/unit-tests-fix-20251105 branch from 269157b to 853812c Compare November 5, 2025 12:02
@kchibisov
Copy link
Member

kchibisov commented Nov 5, 2025

@kchibisov That's what done by cargo +nightly fmt, not me. Lemme remove that commit from this PR.

It doesn't do that for me with latest nightly on master. (We also run it on CI). maybe some rustfmt.toml collision?

@ShikiSuen
Copy link
Contributor Author

ShikiSuen commented Nov 5, 2025

@kchibisov It might be something messed up by my previously-installed old version of Rust. Let's forget about it for now.

kchibisov
kchibisov requested changes Nov 5, 2025
View reviewed changes
Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other than that, it's fine.

@ShikiSuen
Copy link
Contributor Author

ShikiSuen commented Nov 5, 2025

@kchibisov I guess this PR is now in a mergeable state.

@ShikiSuen ShikiSuen requested a review from kchibisov November 5, 2025 13:22
@kchibisov
Copy link
Member

kchibisov commented Nov 6, 2025

You did the opposite of what I've asked for.

You should keep code, but drop changelog.md change.

@ShikiSuen
appkit: patch a unit test issue in monitor.rs regarding display ID.
a55ef9b 
On macOS, make AppKit monitor handle tests derive their reference display IDs from
  `CGMainDisplayID`, preventing failures on systems where the primary monitor is not display `1`.
@ShikiSuen ShikiSuen force-pushed the shiki/unit-tests-fix-20251105 branch from 5782c80 to a55ef9b Compare November 6, 2025 05:07
@ShikiSuen
Copy link
Contributor Author

ShikiSuen commented Nov 6, 2025

You should keep code, but drop changelog.md change.

@kchibisov Done. Sorry for the inconvenience.

@kchibisov kchibisov merged commit d837c88 into rust-windowing:master Nov 6, 2025
57 checks passed
@ShikiSuen ShikiSuen deleted the shiki/unit-tests-fix-20251105 branch November 6, 2025 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kchibisov kchibisov kchibisov approved these changes

@madsmtm madsmtm Awaiting requested review from madsmtm madsmtm is a code owner

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ShikiSuen @kchibisov